feat: return the distance a vector query ranked by, and let the index answer it - #929
Conversation
A vector query could only ever tell a caller the order rows came back in. The distance itself was computed in the ORDER BY clause and thrown away, so there was no way to read a similarity score out of find(). That rules out every relevance-score UI, and it rules out a caller thresholding its own results, because top-N is the only handle available. Project the distance alongside the row and hydrate it onto the document as $distance. Cosine gives 1 - similarity, euclidean gives L2, and dot gives the negative inner product, matching the operator each one orders by, so the number always agrees with the position the row was returned in. The ORDER BY expression is untouched, so planning and index selection are exactly as before. The projected copy is carried as text. A distance is undefined for a zero vector and can overflow for a large one, and pgvector answers NaN in both cases; fetching that straight into a PHP float raises "unexpected NAN value was coerced to string" and takes down the whole query. Reading it as text and interpreting it during hydration yields null for a distance that has no value, rather than the 0.0 a plain float cast produces, which would claim the vectors were identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 54 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe SQL adapter now generates vector-distance ordering and projections. Query results expose distance through ChangesVector distance results
PostgreSQL permission filtering
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant VectorTests
participant Database_find
participant SQL_find
VectorTests->>Database_find: Execute vector search
Database_find->>SQL_find: Build vector ordering and projection
SQL_find-->>Database_find: Return _distance result
Database_find-->>VectorTests: Hydrate Database::VECTOR_DISTANCE
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR exposes raw vector-query distances on returned documents and adjusts PostgreSQL ordering and permission filtering so vector indexes can satisfy searches more efficiently.
Confidence Score: 4/5The PR is not yet safe to merge because the outstanding The shared SQL hydration path still treats any Files Needing Attention: src/Database/Adapter/SQL.php Important Files Changed
Reviews (4): Last reviewed commit: "Update src/Database/Adapter/Postgres.php" | Re-trigger Greptile |
…er it find() appends $sequence to the order attributes whenever nothing unique is already ordering the result. A vector index holds one sort key, so a second one does not merely make the index look expensive, it makes it unusable: the planner has no way to satisfy "distance, then sequence" from a structure that only knows distance. Priced against a sequential scan it still refuses the index, which is what distinguishes this from a costing preference. Every vector search was therefore reading the whole collection and sorting it. On 50k rows of 300 dimensions with an hnsw_cosine index, a top-25 search goes from a 25.3ms parallel sequential scan to a 0.235ms index scan. The tie break exists to hold a page boundary still across a cursor, so keep it when a cursor is present and drop it otherwise. Ties in a float distance over a real embedding are close to unreachable anyway, and an approximate index is free to answer them in either order. A collection carrying its own document permissions is unaffected, because the permissions subquery keeps the planner on a hash semi join. That one is a costing decision rather than a block, since pricing out sequential scans recovers the index scan, and rewriting the subquery as EXISTS does not change the plan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every read resolves permissions through a semi join against the collection's permissions table. A join has to be resolved before anything can be ordered, so whenever the ordering could have come from an index the planner reads the whole collection instead. For a vector search that is the difference between a 400k row sequential scan and touching the index: 119ms to 1.72ms here. The row already carries the same fact. _permissions is written alongside the permissions table on create, bulk create and update, and find() already reads it back to answer $permissions. It was simply not queryable: TEXT, unindexed, output only. Making it JSONB with a GIN index turns the fact we already store into one the planner can cost against the ordering. This is the shape Mongo has always used, where _permissions is matched in the document. Containment rather than the ?| key operator, one per role: PDO reads a lone ? as a positional placeholder and refuses to mix it with named ones, and the ?? escape breaks once a named placeholder repeats, which cursor conditions do. jsonb_exists_any expresses it in a single call but is not an indexable clause and falls back to reading the table, whereas @> is answered as a BitmapOr. Postgres only. The other adapters keep the semi join, so nothing about their plans changes. Matching is byte exact in both forms, so no permission that resolved before resolves differently now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/e2e/Adapter/PostgresTest.php (1)
144-190: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winExercise permission filtering in the vector index test.
Lines 144-147 disable document security and grant collection-level read access to
Role::any().Database::find()then skips adapter authorization. The vector query at lines 176-179 does not executePostgres::getSQLPermissionsCondition().Add a vector query with document-level read enforcement. Assert that unreadable documents are excluded and that the HNSW index scan increases. This verifies the combined vector-ordering and permission-filter contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/Adapter/PostgresTest.php` around lines 144 - 190, Extend the vector index test around the existing vectorPlan setup and find call to enable document security and use document-level permissions, then add a vector query that exercises permission filtering. Assert unreadable documents are excluded while the result remains correctly vector-ordered, and verify the HNSW index scan count increases for this permission-filtered query.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Database/Adapter/Postgres.php`:
- Around line 1835-1840: Update the permission-expression logic around the
$permissions construction to detect an empty $roles list and return the SQL
FALSE predicate before imploding the permissions. Preserve the existing OR
expression for non-empty role lists, ensuring the generated query never contains
empty parentheses.
---
Nitpick comments:
In `@tests/e2e/Adapter/PostgresTest.php`:
- Around line 144-190: Extend the vector index test around the existing
vectorPlan setup and find call to enable document security and use
document-level permissions, then add a vector query that exercises permission
filtering. Assert unreadable documents are excluded while the result remains
correctly vector-ordered, and verify the HNSW index scan count increases for
this permission-filtered query.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b1cd6d7-e983-45e5-85fc-18d6f95be4de
📒 Files selected for processing (2)
src/Database/Adapter/Postgres.phptests/e2e/Adapter/PostgresTest.php
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Two commits. The first returns the distance a vector query ranked by; the second makes that query answerable from the vector index instead of a full scan.
1. Return the distance
A vector query could only tell a caller the order rows came back in.
getVectorDistanceOrder()computed the distance into theORDER BYclause and threw it away, and the existing tests only ever asserted on result order, never on a value.That means there is no way to read a similarity score out of
find(). It rules out any relevance-score UI, and it rules out a caller thresholding its own results ("only matches above 0.8"), because top-N is the only handle available.The distance is now projected alongside the row and hydrated onto the document as
Database::VECTOR_DISTANCE($distance):The value is the raw output of the operator the query ordered by: cosine gives
1 - similarity, euclidean gives L2, dot gives the negative inner product. Kept raw rather than normalised to a 0-1 score so that one invariant holds:$distancealways agrees with the position the row was returned in. Negating the dot product to make "higher is better" would break that.Non-finite distances
Cosine distance to a zero vector divides by a zero magnitude, and a large vector can overflow, so pgvector answers
NaN. Previously that NaN only ever lived insideORDER BYand never crossed into PHP. Projecting it surfaced a real failure:That takes down the entire query, not just the one value.
testVectorAllZeros,testVectorCosineSimilarityDivisionByZeroandtestVectorLargeValuesall caught it.The projected copy is therefore carried as text and interpreted during hydration, so a distance with no value reads back as
null. Note the alternative is worse than an error: a plain(float)cast turns'NaN'into0.0, which tells the caller the two vectors are identical. There is a test pinning that.2. Let the index answer the query
find()appends$sequenceto the order attributes whenever nothing unique is already ordering the result. A vector index holds one sort key, so a second one does not merely make the index look expensive, it makes it unusable — there is no way to satisfy "distance, then sequence" from a structure that only knows distance.Every vector search was therefore reading the whole collection and sorting it. Measured on 50k rows of 300 dimensions with an
hnsw_cosineindex, top-25:Sort+Parallel Seq ScanIndex Scan using words_embedding_idxThe tie break exists to hold a page boundary still across a cursor, so it is kept when a cursor is present and dropped otherwise. Ties in a float distance over a real embedding are close to unreachable, and an approximate index is free to answer them in either order regardless.
3. Match read permissions against the row instead of joining
Every read resolved permissions through a semi join against the collection's permissions table. A join has to be resolved before anything can be ordered, so whenever the ordering could have come from an index, the planner read the whole collection instead. That is what kept a document-secured vector search on a sequential scan no matter how large the collection got.
The row already carries the same fact.
_permissionsis written alongside the permissions table on create, bulk create and update, andfind()already reads it back to answer$permissions. It simply was not queryable:TEXT, unindexed, output only. Making itJSONBwith a GIN index turns a fact we already store into one the planner can cost against the ordering. This is the shape the Mongo adapter has always used, where_permissionsis matched inside the document rather than joined.Measured end to end through
find(), 400k rows of 300 dimensions, document security on, top-25:It also degrades correctly. A permissive permission is a cheap filter over whichever index the ordering wanted; a selective one drives from the GIN index and sorts the small matching set exactly. The planner picks between them because both predicates are finally on the same relation.
Containment (
@>) rather than jsonb's?|key operator, one per role. PDO reads a lone?as a positional placeholder and refuses to mix it with named ones, and the??escape breaks once a named placeholder repeats, which the cursor conditions do.jsonb_exists_anyexpresses the test in a single call but is not an indexable clause and falls back to reading the table;@>is answered as aBitmapOracross roles.Postgres only. The other adapters keep the semi join, so nothing about their plans changes. Matching is byte exact in both forms (verified: the existing
_permissioncolumn has noCOLLATEand the database collation isen_US.utf8, so it was already case sensitive, as@>is), so no permission that resolved before resolves differently now.Safe to do as a plain schema change because Postgres has no users yet, so there is nothing to migrate.
What this still does not fix
Forcing an HNSW scan under a selective filter remains unsafe in general: the scan emits at most
hnsw.ef_searchcandidates (40 by default) and stops, so on 20k rows with 100 readable a forced index scan returns 0 of 25 rows, silently.hnsw.iterative_scan(pgvector 0.8, off by default) fixes that. This is not reachable through normal planning today, before or after these changes: with a selective indexed filter the planner uses that index and sorts exactly, and with an unindexed filter at 1% or 99% selectivity it sequentially scans and sorts, returning 25 of 25 every time. The short read only appears when the index is forced, which the library never does. Worth knowing before anyone reaches forenable_seqscan.Tests
testVectorDistanceintests/e2e/Adapter/Scopes/VectorTests.php, 32 assertions:0.0, orthogonal →1.0, opposite →2.0, and cosine ignores magnitude where euclidean does not<#>returns the negative inner productfind()with no vector query carries no$distanceQuery::select(), which builds a different projectionnull, not a numbertestVectorSearchUsesTheIndexintests/e2e/Adapter/PostgresTest.phpasserts the index is actually scanned, by reading thepg_stat_user_indexesscan counter across the query rather than by inspecting the emitted SQL. It prices sequential scans out of the session first, so it distinguishes the index cannot answer this ordering from the planner found something cheaper, which also keeps it independent of row count.Every part was verified red before being fixed:
unexpected NAN value was coerced to stringFailed asserting that 0.0 is nullA vector search must be answerable from the vector index(0 index scans)Full e2e suites green: Postgres 659, MySQL 658, MariaDB 658, SQLite 658, unit 409. Pint PSR-12 and PHPStan level 7 clean.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes